Skip to content

Conversation

@george-palmsens
Copy link
Contributor

@george-palmsens george-palmsens commented Jul 28, 2025

Address #1277 - STM32 drivers that are not the F4 do not compile NetworkInterface.c because of incorrect naming of ETH_TxPacketConfigTypeDef (should be ETH_TxPacketConfig).

Description

Rename ETH_TxPacketConfigTypeDef to ETH_TxPacketConfig in all drivers.

Test Steps

It didn't compile before, it does compile now. And on our hardware it correctly accepts connections and communicates back and forth.

Checklist:

  • I have tested my changes. No regression in existing tests.

Related Issue

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@george-palmsens george-palmsens requested a review from a team as a code owner July 28, 2025 07:58
@kstribrnAmzn
Copy link
Member

Thanks for opening this PR! I'm taking a look now.

@kstribrnAmzn
Copy link
Member

I took at the latest driver here it seems like it would be better to swap the H4 to name the structure ETH_TxPacketConfigTypeDef instead of modifying all the other ports. If you do this you should only have to rename the F4 port files, legacy port files, and this call.

What are your thoughts?

@george-palmsens
Copy link
Contributor Author

I took at the latest driver here it seems like it would be better to swap the H4 to name the structure ETH_TxPacketConfigTypeDef instead of modifying all the other ports. If you do this you should only have to rename the F4 port files, legacy port files, and this call.

What are your thoughts?

Thanks for taking a look. My gut says the legacy drivers should not be changed, just so they look as much as possible like their old selves.

But I have no strong preference.

@tony-josi-aws
Copy link
Member

@george-palmsens Thanks for the PR.

The latest updated version looks good to me; as @kstribrnAmzn suggested, it seems like ST is making the structure name consistent across different HW platforms (for example, previously the v1.0.0 version of STM32H5 HAL had the struct named as ETH_TxPacketConfig, newer versions name it ETH_TxPacketConfigTypeDef).

The reason for keeping the ST HAL drivers along the network interfaces in the FreeRTOS+TCP repo was primarily due to the inconsistencies across the STM32 HAL, so that anyone who likes to change the driver version could use them as a reference. [refer]

My gut says the legacy drivers should not be changed

I agree with that; the legacy network interfaces of STM32 platforms are supposed to be used with the driver provided along with the network interfaces (it has slight fixes/modifications to work with those legacy interfaces). It's better to keep it as it is.

@aggarg aggarg merged commit 913b90c into FreeRTOS:main Jul 29, 2025
10 checks passed
@george-palmsens
Copy link
Contributor Author

@tony-josi-aws Would you expect this will make it into a release soon? Or do I need to just use master for now?

@tony-josi-aws
Copy link
Member

Yes, this will be included in an upcoming release. While I cannot provide specific timing details at this moment, I recommend using the main branch to proceed with your project. You can then transition to the official release version once it becomes available.

@george-palmsens george-palmsens deleted the fix_1277 branch August 14, 2025 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants